Skip to content

Fix Slack notifications: 'fields' is not a Block Kit block type - #474

Open
richard-devbot wants to merge 1 commit into
mainfrom
fix-slack-block-kit-fields-block-type
Open

Fix Slack notifications: 'fields' is not a Block Kit block type#474
richard-devbot wants to merge 1 commit into
mainfrom
fix-slack-block-kit-fields-block-type

Conversation

@richard-devbot

@richard-devbot richard-devbot commented Jul 27, 2026

Copy link
Copy Markdown
Owner

Problem

Slack notifications have never been deliverable. Both payload builders in src/notifications/index.js (formatSlackStageMessage, formatSlackTaskReportMessage) emit a block of:

{ type: 'fields', fields }

Block Kit has no fields block type — fields belong on a section block. Slack rejects the entire attachment:

Webhook post failed with status: 400. Body: invalid_attachments

Because the failure is at the attachment level, no part of the message is delivered. Notifications are also fire-and-forget by design (router.js), so this failed silently in normal operation.

Reproduction

Against a live Incoming Webhook, with RSTACK_SLACK_WEBHOOK set:

$ npx rstack-agents notify --test
[rstack] Configured channels: slack
  ✗ slack — Webhook post failed with status: 400. Body: invalid_attachments

After this change, same webhook:

$ npx rstack-agents notify --test
[rstack] Configured channels: slack
  ✓ slack

Message confirmed delivered to the channel.

Fix

Producer now emits type: 'section' with fields, which is the correct Block Kit shape.

The bogus type had propagated to every consumer, so each is updated to read fields off a section block while still tolerating the old 'fields' shape — that keeps any persisted or in-flight payloads converting correctly:

File Role
src/notifications/index.js producer — 2 sites
channels/teams.js facts extraction
channels/discord.js embed fields
channels/text.js plain-text flattening

Verification

  • Teams and Discord converters produce identical output to before the change (same 4 facts / 4 embed fields for a stage message with a cost detail).
  • tests/notifications-channels.test.js, tests/harness-notifications.test.js, tests/notify-hook.test.js21/21 pass.
  • The existing slackPayloadToText renders blocks as readable plain text test caught text.js, a consumer missed on the first pass — worth noting the suite did its job here.

Note for maintainers

Separately from this fix: nothing loads a project's .env into process.env. resolveChannels reads process.env and .rstack/notifications.json only, while core/harness/env-file.js is used solely by the Hub UI to edit keys. Following the docs/integrations/webhooks.md guidance to put RSTACK_SLACK_WEBHOOK in .env therefore does not enable the channel unless the variable is also exported into the real environment. Left out of scope here — happy to open a separate issue.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved Slack notification formatting across Discord, Microsoft Teams, and text outputs.
    • Supports field content included in section blocks, including older or nonstandard Slack payloads.
    • Corrected generated Slack Block Kit formatting to use valid section blocks, ensuring fields display reliably.

Slack notifications have never been deliverable. Both payload builders in
src/notifications/index.js emitted a block of `{ type: 'fields', fields }`,
but Block Kit has no `fields` block type — fields belong on a `section`
block. Slack rejects the entire attachment with:

    400 invalid_attachments

Verified against a live Incoming Webhook: failing before this change,
delivering successfully after it.

The bogus type had propagated to every consumer, so each one is updated to
read fields off a section block while still tolerating the old shape, which
keeps any in-flight or persisted payloads converting correctly:

  - channels/teams.js   - facts extraction
  - channels/discord.js - embed fields
  - channels/text.js    - plain-text flattening (covered by the existing
                          "slackPayloadToText renders blocks as readable
                          plain text" test, which caught this consumer)

Teams and Discord converters produce identical output to before the change.
tests/notifications-channels.test.js, harness-notifications.test.js and
notify-hook.test.js pass 21/21.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@strix-security

strix-security Bot commented Jul 27, 2026

Copy link
Copy Markdown

Strix Security Review

No security issues found.

Updated for d82f3c1.


Reviewed by Strix
Re-run review · Configure security review settings

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 9c8e4895-48cf-46c7-ba74-e651e8fd448b

📥 Commits

Reviewing files that changed from the base of the PR and between cce8808 and d82f3c1.

📒 Files selected for processing (4)
  • src/notifications/channels/discord.js
  • src/notifications/channels/teams.js
  • src/notifications/channels/text.js
  • src/notifications/index.js

📝 Walkthrough

Walkthrough

Slack notification formatters now emit valid section blocks containing fields, while Discord, Teams, and text conversions accept fields from both section and legacy fields blocks.

Changes

Slack fields compatibility

Layer / File(s) Summary
Use section blocks for formatted fields
src/notifications/index.js
Stage and task report formatters now place fields on section blocks instead of using the nonstandard fields block type.
Convert fields from section blocks
src/notifications/channels/discord.js, src/notifications/channels/teams.js, src/notifications/channels/text.js
Channel converters now process fields attached to both section and fields blocks, preserving compatibility with older payload shapes.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: richardsongunde

Poem

I’m a rabbit with blocks tucked neat,
Section fields now hop into the right seat.
Discord, Teams, and text all agree,
Old shapes still travel happily.
Ears up for cleaner payloads—whee!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main fix: replacing an invalid Slack Block Kit fields block with section blocks carrying fields.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-slack-block-kit-fields-block-type

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix Slack Block Kit payloads: move fields onto section blocks

🐞 Bug fix 🕐 20-40 Minutes

Grey Divider

AI Description

• Fix Slack payload blocks by using type: "section" for fields.
• Update Teams/Discord/Text converters to read fields from section (legacy-tolerant).
Diagram

graph TD
  Fmt["Slack payload builder"] --> Slack["Slack webhook"]
  Fmt --> DiscordConv["Discord converter"] --> Discord["Discord webhook"]
  Fmt --> TeamsConv["Teams converter"] --> Teams["Teams webhook"]
  Fmt --> TextConv["Text flattener"] --> Text["Plain-text channels"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Normalize blocks via a shared helper (e.g., `getBlockFields(block)`)
  • ➕ Eliminates repeated (section || fields) checks across consumers
  • ➕ Centralizes backward-compat handling and makes future schema tweaks safer
  • ➖ Slightly more abstraction/indirection for a small codebase
  • ➖ Requires touching more files and exporting a helper API
2. Add Block Kit schema validation before sending to Slack
  • ➕ Catches invalid payloads early and makes failures explicit
  • ➕ Prevents future regressions beyond just the fields issue
  • ➖ Adds runtime cost and dependency/maintenance surface
  • ➖ Doesn’t address non-Slack consumers unless they also validate

Recommendation: The PR’s approach (fix producer + make consumers tolerant) is the best immediate fix: it restores Slack delivery while preserving compatibility for any persisted/in-flight payloads. If this pattern repeats, consider introducing a small shared normalization helper to avoid duplicating legacy-shape checks.

Files changed (4) +10 / -5

Bug fix (4) +10 / -5
index.jsEmit Block Kit 'section' blocks for 'fields' payloads +4/-2

Emit Block Kit 'section' blocks for 'fields' payloads

• Fixes both Slack payload builders to use 'type: "section"' for blocks that carry 'fields', matching Slack Block Kit’s required shape. Adds clarifying comments explaining why the previous 'fields' block type was invalid.

src/notifications/index.js

discord.jsParse 'fields' from 'section' blocks (legacy-tolerant) +2/-1

Parse 'fields' from 'section' blocks (legacy-tolerant)

• Updates Slack→Discord conversion to treat 'fields' as belonging to 'section' blocks while still accepting legacy blocks with 'type: "fields"'. This preserves embed field extraction for older payloads.

src/notifications/channels/discord.js

teams.jsExtract facts from 'section.fields' (legacy-tolerant) +2/-1

Extract facts from 'section.fields' (legacy-tolerant)

• Adjusts Slack→Teams conversion to read 'block.fields' from 'section' blocks, while continuing to accept the old 'fields' block type. Keeps MessageCard facts output consistent across payload versions.

src/notifications/channels/teams.js

text.jsFlatten 'section.fields' into plain text (legacy-tolerant) +2/-1

Flatten 'section.fields' into plain text (legacy-tolerant)

• Updates plain-text rendering to treat 'fields' as belonging to 'section' blocks, while tolerating the legacy 'fields' block type. Prevents missing details when converting updated Slack payloads into text-only output.

src/notifications/channels/text.js

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (1)

Context used
✅ Compliance rules (platform): 300 rules
✅ Skills: 17 invoked
  code-review-pr
  claude-api
  documentation-writing
  pptx
  docx
  performance-monitoring
  security-compliance
  cso
  plan-eng-review
  design-review
  prompt-engineering
  mcp-builder
  qa-testing
  code-patterns
  xlsx
  security-owasp
  testing-qa

Grey Divider


Remediation recommended

1. Legacy fields branch untested 📜 Skill insight ▣ Testability
Description
A new backward-compatibility branch was added so Slack block parsing accepts both section blocks
with fields and legacy block.type === 'fields', but the existing notification channel tests only
exercise the section path. This leaves the legacy conversion path unverified and risks regressions
for persisted/in-flight payloads shipping unnoticed.
Code

src/notifications/channels/discord.js[R24-26]

+      // Fields ride on a section block; 'fields' is tolerated for older payloads.
+      if ((block.type === 'section' || block.type === 'fields') && block.fields) {
        for (const f of block.fields) {
Relevance

⭐⭐⭐ High

Team often accepts adding regression tests for uncovered branches/edge cases; legacy compatibility
path should be verified.

PR-#399

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Compliance rules (Rule 1400055 / PR Compliance ID 1399871) require tests to cover both sides of new
or modified conditional branches. The converters now explicitly include a branch for legacy
block.type === 'fields', but the tests construct Slack payloads through formatSlackStageMessage,
which now emits type: 'section' for the fields block; as a result, only the section side of the
condition is exercised and the legacy fields branch is not covered.

src/notifications/channels/discord.js[24-33]
src/notifications/channels/teams.js[27-40]
src/notifications/channels/text.js[26-34]
tests/notifications-channels.test.js[19-42]
src/notifications/index.js[83-96]
src/notifications/channels/text.js[26-28]
Skill: plan-eng-review
Skill: code-review-pr

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Converters now accept both `section` blocks with `fields` and legacy `block.type === 'fields'`, but current tests only exercise the new `section` shape via `formatSlackStageMessage`, leaving the backward-compatibility branch untested.

## Issue Context
This PR explicitly aims to tolerate older persisted/in-flight Slack payloads; without a dedicated test that includes a legacy `{ type: 'fields', fields: [...] }` block, that compatibility guarantee can regress silently. Add a test that feeds a legacy Slack payload using `type: 'fields'` into each converter (e.g., `slackPayloadToText`, `convertSlackToDiscord`, `convertSlackToTeams`) and asserts the expected extracted fields / outputs.

## Fix Focus Areas
- tests/notifications-channels.test.js[19-42]
- src/notifications/channels/discord.js[20-33]
- src/notifications/channels/teams.js[17-40]
- src/notifications/channels/text.js[21-34]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment on lines +24 to 26
// Fields ride on a section block; 'fields' is tolerated for older payloads.
if ((block.type === 'section' || block.type === 'fields') && block.fields) {
for (const f of block.fields) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

1. Legacy fields branch untested 📜 Skill insight ▣ Testability

A new backward-compatibility branch was added so Slack block parsing accepts both section blocks
with fields and legacy block.type === 'fields', but the existing notification channel tests only
exercise the section path. This leaves the legacy conversion path unverified and risks regressions
for persisted/in-flight payloads shipping unnoticed.
Agent Prompt
## Issue description
Converters now accept both `section` blocks with `fields` and legacy `block.type === 'fields'`, but current tests only exercise the new `section` shape via `formatSlackStageMessage`, leaving the backward-compatibility branch untested.

## Issue Context
This PR explicitly aims to tolerate older persisted/in-flight Slack payloads; without a dedicated test that includes a legacy `{ type: 'fields', fields: [...] }` block, that compatibility guarantee can regress silently. Add a test that feeds a legacy Slack payload using `type: 'fields'` into each converter (e.g., `slackPayloadToText`, `convertSlackToDiscord`, `convertSlackToTeams`) and asserts the expected extracted fields / outputs.

## Fix Focus Areas
- tests/notifications-channels.test.js[19-42]
- src/notifications/channels/discord.js[20-33]
- src/notifications/channels/teams.js[17-40]
- src/notifications/channels/text.js[21-34]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants